Skip to content

Conversation

michaelabuckley
Copy link
Contributor

@michaelabuckley michaelabuckley commented Jul 30, 2025

Merge Request Description

Summary

This MR refactors the HAPI FHIR Repository search API to improve flexibility and maintainability by introducing a builder pattern for constructing search queries. The changes replace the rigid SearchConverter with a more extensible query building system that properly handles FHIR search parameters including special parameters like sorting, pagination, includes, and summary modes.

  1. New Query Builder API: Introduces IRepositoryRestQueryBuilder interface with SearchParameterMapRepositoryRestQueryBuilder implementation for flexible query construction
  2. API Evolution: Adds new search methods using callback pattern while maintaining backward compatibility through deprecation of old multimap-based methods
  3. SearchConverter Removal: Replaces the monolithic SearchConverter class (145 lines deleted) with specialized parameter processors (122 lines of new, focused code)
  4. Enhanced Parameter Handling: Implements dedicated processors for special FHIR parameters (_sort, _count, _include, _revinclude, etc.) with proper type conversion and validation
  5. Comprehensive Test Coverage: Adds extensive test suites for both the multimap and SearchParameterMap query builders

Code Review Suggestions

  • API Compatibility: Verify that the deprecation strategy properly maintains backward compatibility for existing IRepository implementations while encouraging migration to the new builder pattern
  • Parameter Processing Logic: Pay close attention to the parameter processors in SearchParameterMapRepositoryRestQueryBuilder - ensure all FHIR special parameters (_sort, _include, _count, etc.) are handled correctly and edge cases are covered
  • Default Method Cycle: Review the circular default implementations between the multimap and contributor-based search methods in IRepository - implementors must override at least one method to avoid infinite recursion
  • SearchParameterMap Integration: Examine how SearchParameterMapRepositoryRestQueryBuilder.buildFromQueryContributor() handles the passthrough case when the contributor is already a SearchParameterMap (used for complex operations like $everything)
  • Error Handling: Check that parameter validation and conversion failures are properly handled with meaningful error messages, especially in the special parameter processors

QA Test Suggestions

Setup

  • Set up a test environment with HAPI FHIR JPA server
  • Create test data including Patient, Observation, and other resources with various relationships
  • Ensure both deprecated and new API methods are accessible for testing

Test Cases

  • Basic Search Functionality: Verify that simple resource searches (e.g., Patient?name=John) work identically through both old multimap and new builder APIs
  • Special Parameter Handling: Test all FHIR special parameters including _sort, _count, _offset, _include, _revinclude, _summary, _contained, and _total to ensure they produce equivalent results
  • Complex Query Building: Test complex searches combining multiple parameters, sorting, includes, and pagination to verify the builder pattern constructs equivalent queries to the old multimap approach
  • Error Scenarios: Test invalid parameter values, malformed sort specifications, and other error conditions to ensure proper error handling and meaningful error messages
  • Performance Comparison: Run search performance tests comparing old vs new implementation to ensure no regression in query execution time
  • Backward Compatibility: Test existing client code that uses the deprecated multimap methods to ensure they continue to function without modification

@michaelabuckley michaelabuckley marked this pull request as draft July 30, 2025 00:06
@michaelabuckley michaelabuckley changed the base branch from master to rel_8_4 July 30, 2025 00:06
…itory-search

# Conflicts:
#	hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java
#	hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/HapiFhirRepositoryTest.java
#	hapi-fhir-repositories/pom.xml
#	hapi-fhir-repositories/src/main/java/ca/uhn/fhir/repository/impl/GenericClientRepository.java
#	hapi-fhir-repositories/src/main/java/ca/uhn/fhir/repository/impl/memory/InMemoryFhirRepository.java
#	hapi-fhir-repositories/src/main/resources/META-INF/services/ca.uhn.fhir.repository.IRepositoryLoader
#	hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/repository/HapiFhirRepository.java
#	hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/repository/IRepositoryTest.java
@robogary
Copy link
Contributor

robogary commented Jul 30, 2025

This Pull Request has failed the formatting check

Please run mvn spotless:apply or mvn clean install -DskipTests to fix the formatting issues.

You can automate this auto-formatting process to execute on the git pre-push hook, by installing pre-commit and then calling pre-commit install --hook-type pre-push. This will cause formatting to run automatically whenever you push.

@michaelabuckley michaelabuckley changed the base branch from rel_8_4 to master September 24, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants